Skip to content

fix(core): stabilize collinear wall junction ordering - #596

Merged
Aymericr merged 1 commit into
pascalorg:mainfrom
tomatotomata:codex/deterministic-wall-junction-order-581
Aug 5, 2026
Merged

fix(core): stabilize collinear wall junction ordering#596
Aymericr merged 1 commit into
pascalorg:mainfrom
tomatotomata:codex/deterministic-wall-junction-order-581

Conversation

@tomatotomata

@tomatotomata tomatotomata commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • make exactly-collinear wall junction ordering deterministic by using wall ID as the stable tie-breaker after outgoing angle
  • add a regression test that compares the same wall set in forward and reversed input order

Closes #581

Validation

  • npx --yes bun@1.3.0 test packages/core/src/systems/wall/wall-mitering.test.ts passed: 12 tests, 0 failures, 145 expectations
  • npx --yes bun@1.3.0 x biome lint packages/core/src/systems/wall/wall-mitering.ts packages/core/src/systems/wall/wall-mitering.test.ts passed
  • git diff --check passed

The near-equal-angle epsilon behavior mentioned in the issue is intentionally out of scope.


Note

Low Risk
Small, localized change to junction sort order in wall rendering geometry; behavior only affects equal-angle cases and is covered by a new test.

Overview
Wall miter junctions at shared corners now sort connected walls by outgoing angle and, when angles match (collinear overlaps), by wall ID so adjacent-wall pairing and intersection geometry no longer depend on how walls are ordered in the input array.

A regression test asserts that calculateLevelMiters yields identical junctionData for the same wall set in forward vs reversed order at a multi-wall junction.

Reviewed by Cursor Bugbot for commit 02aae8d. Bugbot is set up for automated code reviews on this repo. Configure here.

@Aymericr

Aymericr commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Approved and merging — thank you @tomatotomata, and welcome. This is the right fix and the right size for it.

Verified rather than assumed:

  • The test genuinely fails on main, and not on map insertion order — I reverted wall-mitering.ts to main while keeping your test, and it fails with real geometry differences (wide's left corner at y: 0.3 vs 0.1, and the mirrored change on narrow's right). That's the pairing flip the issue described, so this test would catch a regression rather than just pass alongside the fix.
  • wallId is the correct tie-break key, and it's what's on ProcessedWall — no plumbing needed. Stable across reload and across peers, unlike thickness, which isn't unique.
  • Passthrough walls push two entries with the same wallId (v1 and v2), so the comparator can still return 0 for a pair. Not a problem: the two entries are 180° apart, so their angles differ and the tie-break is never reached for them. Worth knowing it's true by construction rather than by luck.
  • processedWalls.sort is the only order-sensitive comparator left in the file. The other one at line 214 keys on an explicit wallOrder map, which is already deterministic.
  • The miter cache is safe. I checked level-miter-cache.ts because the issue flagged it: sameMiterInputs compares positionally, so a reordered array is a cache miss, never a stale hit. With this fix the recompute now returns identical data, which is exactly the property the cache wanted.

Full gate green locally: check 1601 files clean, check-types 9/9, test 12/12 tasks (945 nodes tests + 12 mitering), build 7/7.

Agreed on scoping out the near-equal-angle epsilon, and on reflection it belongs out of scope for a stronger reason than "different change": it isn't a determinism gap at all. I raised it in the issue as an open question, and working through your fix answers it. angle is a pure function of the wall's own geometry, so two walls differing by 1e-16 sort consistently every time — the order is stable, it's just not the order a human would call "obvious". What I was actually describing there is miter quality at shallow angles, which MITER_LIMIT already handles by falling back to a square joint. So Closes #581 is right and I'm letting it close; no follow-up needed. An epsilon tie-break would also make the comparator non-transitive, which is undefined behavior for Array.prototype.sort — so it'd be a bucketing change, not a comparator tweak, and there's no reason to make it.

One note on your validation commands: npx --yes bun@1.3.0 test <file> works, but you can run the whole repo gate with bun run check && bun run check-types && bun run test && bun run build — that's what CI runs, and it catches cross-package type breakage that a single-file test run won't. Not needed here; your change was clean through all four.

@Aymericr
Aymericr marked this pull request as ready for review August 5, 2026 03:34
@Aymericr
Aymericr merged commit 163ef0b into pascalorg:main Aug 5, 2026
2 checks passed
@Aymericr

Aymericr commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Merged as 163ef0be, and #581 closed with it.

One process note for next time: this was open as a draft, which is why CI didn't run until I approved the workflow (first-time contributor from a fork needs that regardless) and why the merge initially bounced. The body read as a finished submission — full validation output and an explicit scope note — so I marked it ready myself rather than bouncing it back to you. If a PR is ready for review, opening it undrafted gets it looked at sooner.

Thanks again. If you want another in the same area, #588 (batching a level's static walls per material) is labelled help wanted and is the natural next step in the same wall-geometry path you just worked in.

Aymericr added a commit that referenced this pull request Aug 5, 2026
…fixes (#598)

Both #596 and #597 landed without a CHANGELOG line. Match the 0.6.0 style
and name the contributors, since the release notes are how credit for an
outside fix actually surfaces.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wall junction miter order is non-deterministic for exactly-collinear walls

2 participants